Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: invalid memory address #2585

Merged
merged 2 commits into from
Mar 31, 2023
Merged

fix: invalid memory address #2585

merged 2 commits into from
Mar 31, 2023

Conversation

ShaPoHun
Copy link
Contributor

What type of this PR

Examples of user facing changes:

  • Bug fixes
    "invalid memory address or nil pointer dereference" in vpc_nat.go

Which issue(s) this PR fixes:

Fixes #(issue-number)
#2583

@github-actions
Copy link
Contributor

  • The commit messages for both patches are not informative enough. They only mention "fix: invalid memory address" without any further explanation of what the issue was and how it was fixed. It would be better to provide more details in the commit messages.
  • In the first patch, there is a potential bug if the "image" key does not exist in the ConfigMap data. In that case, the code will still try to assign an empty string to the vpcNatImage variable, which could cause unexpected behavior later on. It would be better to return early from the function if the "image" key is not found.
  • In the second patch, there is a missing check for the "IsNotFound" error. If the error is "IsNotFound", the function should return early instead of logging an error message.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should explain what the patch does and why it is necessary.
  • In the first patch, there is a potential bug in the resyncVpcNatConfig() function. If the "image" key is not found in the ConfigMap data, the vpcNatImage variable will still be set to an empty string, which could cause issues later on. A better approach would be to return an error or use a default value if the key is not found.
  • In the second patch, there is a potential bug in the resyncVpcNatConfig() function. If the error returned by the ConfigMaps() method is "not found", the function returns without doing anything. However, this could cause issues if the ConfigMap is created later on. A better approach would be to log a warning message and continue with the function.

@oilbeater oilbeater added bug Something isn't working need backport labels Mar 31, 2023
@oilbeater oilbeater merged commit 5b7bdcc into kubeovn:master Mar 31, 2023
@ShaPoHun ShaPoHun deleted the fix_invalid_memory_ branch April 6, 2023 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oilbeater oilbeater oilbeater approved these changes

@bobz965 bobz965 bobz965 approved these changes

@hongzhen-ma hongzhen-ma Awaiting requested review from hongzhen-ma

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ShaPoHun @oilbeater @bobz965